I have everything I need. Here is the full review.

Review: PR #194 — ci: use canonical dogfood URL

Files touched: .github/workflows/ci.yml, .github/workflows/dogfood.yml, justfile, tests/ci-canonical-docs-url-validate.sh
Crate buckets: ci, justfile, shell tests (no Rust)

Also reviewed: merged fix(cdp) commit b1d0639 (fix(cdp): handle optional DOMSnapshot string sentinels #191), which is in HEAD and is the most substantive code change touching a library crate.

1. Determinism (PR #194)

No Rust code changes. Not applicable.

2. Determinism (fix(cdp) b1d0639)

crates/plumb-cdp/src/lib.rs:1035–1039 — lookup_optional_string is a pure function of (strings, idx). No wall-clock, no env, no HashMap in observable output (styles and attributes both use IndexMap). Clear.

3. Workspace layering

plumb-cdp imports plumb_core only. #![deny(unsafe_op_in_unsafe_fn)] is present at line 47; no new unsafe blocks introduced in either change. Layering intact.

4. Error handling

crates/plumb-cdp/src/lib.rs:1164–1165:

let value = lookup_optional_string(strings, *chunk[1].inner())?
    .unwrap_or("")

The ? propagates the CdpError; .unwrap_or("") operates on the resulting Option<&str>. This is Option::unwrap_or, not Result::unwrap; clippy::unwrap_used does not flag it. Semantically correct — an absent attribute value is represented as an empty string, matching Chrome's own behavior for present-but-empty attributes.

#![deny(clippy::unwrap_used, clippy::expect_used)] at line 47 is in force. All non-test unwrap calls in the file are unwrap_or/unwrap_or_else. The unwrap() calls at lines 1427–1478 are inside #[cfg(test)]. Compliant.

5. Test coverage (fix(cdp))

Three unit tests at lines 1452–1478 cover:

• Sentinel (-1) and arbitrary negative → None
• Valid in-range index → Some("…")
• Out-of-range → Err(MalformedSnapshot)

That is the full decision surface of lookup_optional_string. Coverage is adequate.

6. Shell script issues (PR #194)

W1 — tests/ci-canonical-docs-url-validate.sh:74–76: dead self-exclusion filter

if [[ "$line" == *"tests/ci-canonical-docs-url-validate.sh:"* ]]; then
    continue
fi

The file on disk contains 'plumb\.dev' (backslash-dot, as a regex literal). rg searches for the pattern plumb\.dev, which matches the literal string plumb.dev (dot only). A backslash-dot in the file content is not plumb.dev, so rg will never produce a match line pointing at this script. The filter is dead code. Harmless, but it implies a self-reference risk that doesn't exist and will confuse future editors.

W2 — tests/ci-canonical-docs-url-validate.sh:69–81: silent rg fallback

The scan for remaining plumb.dev references does rg … || true. On ubuntu-latest, rg is pre-installed, so CI is unaffected. But just check is also a local command; a developer without ripgrep silently skips step 3 and gets a false green. A guard like command -v rg >/dev/null || { echo "ripgrep required (cargo install ripgrep)"; exit 1; } at the top would make the dependency explicit. Neither the other validator script (ci-chrome-sandbox-validate.sh) nor the justfile lists rg as a prerequisite.

These are warnings, not blockers.

7. Documentation

justfile:65 — new recipe has a one-line comment describing its purpose. Consistent with surrounding recipes. docs/src/ci/ was not changed, which is appropriate since this is internal CI wiring, not a user-visible feature.

Punch list

No blockers found in either the PR #194 CI changes or the merged fix(cdp) lookup_optional_string implementation.

Verdict: APPROVE